Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify epoch state diff logging #5863

Merged
merged 1 commit into from
May 31, 2024
Merged

Simplify epoch state diff logging #5863

merged 1 commit into from
May 31, 2024

Conversation

carbolymer
Copy link
Contributor

@carbolymer carbolymer commented May 29, 2024 •
edited
Loading

Description

Simplify epoch state diff logging. This is a follow-up from #5854. Now diffs are logged live.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • roundtrip tests
    • integration tests
      See Runnings tests for more details
  • Any changes are noted in the CHANGELOG.md for affected package
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
  • Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

@carbolymer carbolymer requested a review from a team as a code owner May 29, 2024 11:01
@carbolymer carbolymer force-pushed the mgalazyn/test/epoch-state-diffing branch 6 times, most recently from 2315a3a to add9843 Compare May 29, 2024 21:25
palas
palas approved these changes May 30, 2024
View reviewed changes
Copy link
Contributor

@palas palas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification of the state log diffing mechanism. I am not familiar with ResourceT and Async, so I may have missed some things, even though I did read the post linked. But overall it looks pretty solid to me, I just suggested a couple of minor lints

cardano-testnet/src/Testnet/Runtime.hs Outdated Show resolved Hide resolved
cardano-testnet/src/Testnet/Runtime.hs Outdated Show resolved Hide resolved
cardano-testnet/src/Testnet/Property/Util.hs Outdated Show resolved Hide resolved
@smelc smelc force-pushed the mgalazyn/test/epoch-state-diffing branch 2 times, most recently from 784081c to e74f6fc Compare May 31, 2024 07:50
@smelc smelc enabled auto-merge May 31, 2024 07:52
@smelc smelc force-pushed the mgalazyn/test/epoch-state-diffing branch from e74f6fc to ea1c8ce Compare May 31, 2024 13:01
@smelc smelc added this pull request to the merge queue May 31, 2024
Merged via the queue into master with commit 532cac9 May 31, 2024
24 checks passed
@smelc smelc deleted the mgalazyn/test/epoch-state-diffing branch May 31, 2024 17:08
@carbolymer carbolymer mentioned this pull request Jun 6, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@palas palas palas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@carbolymer @palas @smelc